Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a JS toggle module #740

Merged
merged 4 commits into from
Feb 24, 2016
Merged

Add a JS toggle module #740

merged 4 commits into from
Feb 24, 2016

Conversation

dsingleton
Copy link
Contributor

Most of the work done by @fofr. I cherry-picked it off his branch and made a slight tweak in the second commit (which @fofr was happy with).

This is the first use of JS Modules at the static level of GOV.UK, though the start code was added to support it's use at an application level.

This particular module is for toggling visibility of content and is being added to support the accessibility toggle on attachments, which i'll be adding the govspeak component in a follow up PR.

Example usage of the general pattern:

<div data-module="toggle">
    <a href="#" class="my-toggle" data-expanded="false" data-controls="target">Toggle</a>
    <div id="target">Target</div>
</div>

Includes a bump to govuk_template

Mergable when:

fofr and others added 2 commits February 22, 2016 17:10
* Use the `hidden` class provided by govuk_template to hide and show
content
* Allow multiple targets and toggles
* Use `aria-expanded` and `aria-controls` attributes to determine
starting state and to indicate which toggle controls what
* Toggle elements must have both `aria-expanded` and `aria-controls` to
work
We can detect the control/toggle element from the module data-*
attributes, and don't need to rely on a specific class as well.

This feels more in keeping with the data-module approach we're taking
where the 'interface' to the module is encoded in data-attributes
and their values, and doesn't rely on anything else in the element
passed into the module, eg classes.

This allows the class of the element to be specific to use case in
the app invoking the module.

I've removed the class part of the selector, and updated the tests
so the examples use an example selector that is less likely to be
confused for module configuration.
@fofr
Copy link
Contributor

fofr commented Feb 23, 2016

We might need to use a js- class for hiding and showing for the non-js case.

I'm happy with 6237ab0, leaving for someone else to review the rest though.

Perhaps @robinwhittleton or @tombye?

A conditional hook to hide content if JS is enabled is being added
to `govuk_template`, and is a pattern we use a lot in other apps.

Doing this, rather than hiding the content with the module JS
prevents a flash of visible content between the content loading and
the JS executing.
@dsingleton
Copy link
Contributor Author

PR to add a .js-hidden hook to govuk_template - and i've pre-emptively updated this PR to use it.

Assuming @fofr is happy this can be merged once alphagov/govuk_template#203 is merged, and released, and this PR is updated to use the latest version of govuk_template.

@dsingleton dsingleton changed the title Add a JS toggle module [Do-Not-Merge] Add a JS toggle module Feb 23, 2016
@fofr
Copy link
Contributor

fofr commented Feb 23, 2016

.js-hidden already exists in Whitehall where this will be used first, so it could be merged sooner if the other PR gets blocked.

To get the  hook that the toggle module uses

Version diff: alphagov/govuk_template@v0.16.1...v0.17.0

Relevant changelog:

0.17.0

- Add CSS hook (`.js-hidden`) for hiding content when JS is enabled. Some apps
  have an equivalent hook, which can be removed once upgraded to this version

0.16.4

- Fix publish the Jinja version of the template with a `package.json` for those
  consuming it with NPM

0.16.3

- make the Django version of the template into a proper Python package
- publish the Jinja version of the template with a `package.json` for those
  consuming it with NPM

0.16.2

- more static assets added to `assets.precompile` to improve
  compatibility with apps running rails > 4.2.5
@dsingleton dsingleton changed the title [Do-Not-Merge] Add a JS toggle module Add a JS toggle module Feb 24, 2016
@dsingleton
Copy link
Contributor Author

.js-hidden now comes from a bumped version of govuk_template.

benlovell added a commit that referenced this pull request Feb 24, 2016
@benlovell benlovell merged commit 7ee7e18 into master Feb 24, 2016
@benlovell benlovell deleted the toggle-module branch February 24, 2016 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants